nit: If that's the only difference let's say create_channel_to_trusted_peer_0_reserve? Nice to be explicit, imo.

Two questions. Shouldn't we check that if a channel has the 0-reserve feature bit and if it is fail if the user isn't accepting 0-reserve? Also why shouldn't we just set it now? I'm not sure we need to bother with a staging bit, really, honestly...

Can we re-use check_no_outputs here? Could also return early if we don't need any action.

We'd need to change it to a saturating_sub, but I think that's okay because we have a checked_sub check of our balances on 321 after calling check_no_outputs anyway?

done below thank you !

Comment for (1) says "at the spiked feerate" but isn't tx_fee_sat is just our current feerate?

A more descriptive name would be helpful here (if you can thing of something less gross than max_output_preserving_fee?).

Comment for (1) says "at the spiked feerate" but isn't tx_fee_sat is just our current feerate?

See the callsites of the function; we always use the spiked_feerate as the feerate_per_kw parameter. I admit this is confusing, should I rename the function parameter to spiked_feerate ?

A more descriptive name would be helpful here (if you can thing of something less gross than max_output_preserving_fee?).

How is current_spiked_tx_fee_sat ? It would be consistent with the spiked_feerate style. With max_output_preserving_fee you want to highlight that we want to maintain an output on the commitment transaction even with a 2x increase in the feerate ?

went ahead with both variable renames below

Shouldn't we be setting the option_zero_reserve feature in lightning/bolts#1140?

Discussed offline, I'll hold off on signaling for now pending further spec discussions

(first time I posted this I think it got lost in the depths of GH).

Can we live without some of these helpers, because this accept step is the only one where we need to take a custom action?

Will add a few lines in do_test_accept_inbound_channel_from_trusted_peer_0reserve but the others are unaffected?

done below thank you

Can we pull these asserts out of the channel type branching? If we use the correct fee rate / anchor values for each different type (eg, zero for 0FC) then they're the same?

See the fixups below, let me know if the test now reads better. I made some overall cleanups to help with readability.

Same here I think? Asserts are common for sender_amount_msat, we may just sometimes have zero anchors/fees somtimes.

See the fixups below, as described above, I made a bigger reorg of the tests.

We've switched the dust limits used to floor channel reserve here - I think the counterparty's reserve should use the common_fields.dust_limit_satoshi and we should use MIN_CHAN_DUST_LIMIT_SATOSHIS for holder?

We've switched the dust limits used to floor channel reserve here

Yes claude's review has been screaming at me asking for a separate commit, will do this if you are good too.

I think the counterparty's reserve should use the common_fields.dust_limit_satoshis

The counterparty's reserve / holder_selected_channel_reserve_satoshis, should not be smaller than the counterparty's dust limit, msg.common_fields.dust_limit_satoshis here.

we should use MIN_CHAN_DUST_LIMIT_SATOSHIS for holder?

The holder's reserve / counterparty_selected_channel_reserve_satoshis, should not me smaller than the holder's dust limit, MIN_CHAN_DUST_LIMIT_SATOSHIS here.

Let me know if we agree, I believe the current diff is correct.

Nice changes here 👌

PaymentSucceeds is the only test case that's common to all of our channel types, and FailsReceiverUpdateAddHTLC has its own branch within the failure case here.

Wondering if these can be separated into different handlers with common setup to improve readability? Would also save our needing to check the channel type / NoOutputs combination above - just assert that ReceiverCanAcceptA/B is only running on static_remote_key, for example.

See the fixups below, these would all get squashed into the test commit.

Bug: FundingScope::new_for_splice preserves zero reserves from the previous funding, but validate_splice_contributions (line 12303) always recalculates reserves using get_v2_channel_reserve_satoshis(post_channel_value, ...), which never returns 0. This means splice validation is stricter than the actual reserve for zero-reserve channels.

Concrete scenario: if a zero-reserve channel party has a post-splice balance less than the standard v2 reserve (e.g., 500 sats when the v2 reserve calculates to 546+), the splice will be rejected at validation time, even though the actual post-splice FundingScope would have reserve=0 and the splice would be valid.

validate_splice_contributions should check the previous funding's reserve values and propagate zero reserves the same way new_for_splice does.

nit: Two adjacent bool parameters (accept_0conf, accept_0reserve) are easy to swap accidentally at call sites. Consider a small builder/options struct or at minimum adding // accept_0conf, accept_0reserve comments at every call site. Several existing call sites (e.g., chanmon_update_fail_tests.rs:3239) already read 0, true, false, None which is not self-documenting.

Bug: accept_0reserve is silently ignored for V2/dual-funded channels. When the code path at the V2 branch (around line 10921) calls PendingV2Channel::new_inbound, it doesn't pass accept_0reserve — that function hardcodes is_0reserve = false (channel.rs line 14373-14375).

A user calling accept_inbound_channel_from_trusted_peer(id, node, 0, false, /*accept_0reserve=*/true, None) on a V2 channel open will believe they've set up zero-reserve, but the channel will be created with a normal 1% reserve.

Either:

• Pass accept_0reserve through to PendingV2Channel::new_inbound and wire it into the reserve calculation, or
• Return an APIError::APIMisuseError when accept_0reserve == true and the channel is V2, so the caller knows their intent was not honored.

nit: The $trusted_accept branch calls accept_inbound_channel_from_trusted_peer with (accept_0conf=false, accept_0reserve=true), which means the zero-reserve acceptance path never exercises zero-conf. Consider also fuzzing the combination (true, true) on at least one channel to cover the zero-conf + zero-reserve interaction path that test_zero_reserve_zero_conf_combined tests.

leaving this to a follow-up to this PR

Bug: The signature change from bool to Option<TrustedChannelFeatures> here was not propagated to the two #[cfg(ldk_test_vectors)] test functions that call OutboundV1Channel::new:

1. outbound_commitment_test() — currently passes false (around line 17032)
2. zero_fee_commitment_test_vectors() — currently passes false (around line 17757)

Both need to be updated to pass None instead of false. This won't be caught in normal CI since the tests are gated on #[cfg(ldk_test_vectors)], but will cause a compilation failure when running test vectors.

nit: Consider adding #[derive(Clone, Copy)] (or at least Copy) on this fieldless enum. While Rust allows matching without Copy on fieldless enums (it reads only the discriminant), having Copy makes the intent explicit and avoids confusion for future maintainers. It also enables patterns like let ct = chan_type; if ever needed.

nit: The #[rustfmt::skip] removal on new_for_inbound_channel and new_for_outbound_channel reformats ~500 lines in the diff, burying the actual semantic changes (the trusted_channel_features parameter, zero-reserve validation relaxation, and minimum_depth logic). Consider splitting the formatting into a separate commit (the last commit message mentions formatting create_channel_internal but not these two functions).

I don't think it is good to build out this way of increasing coverage. If chan type is part of the fuzz bytes, the fuzzer is able to zoom in on type-specific code paths.

thanks for taking a look ! see the commit below

looks good

The previous code ran do_test twice per fuzz input — once with anchors=false, once with anchors=true — guaranteeing every corpus entry exercised both channel type families. The new code runs do_test only once, with the channel type extracted from the fuzz input byte.

This means existing fuzz corpus entries that were evolved to cover interesting states under the two-pass model now only exercise a single channel type (determined by whatever happens to be in bits 3-4 of byte 0). Corpus entries that were valuable because they triggered anchor-specific behaviour may now be routed to the Legacy path or vice-versa, effectively discarding coverage.

Consider keeping the multi-pass structure by iterating over ChanType variants, or at minimum re-minimizing the corpus after this change.

create_channel_to_trusted_peer_0reserve hardcodes TrustedChannelFeatures::ZeroReserve, meaning there's no API for opening an outbound channel with both zero-conf and zero-reserve. The accept side has ZeroConfZeroReserve, but the open side has no counterpart.

This asymmetry means a user who wants to create a zero-reserve channel and also trust the counterparty for zero-conf funding (e.g., a mutual trust scenario) can't do so from the opener side. They'd need to call create_channel_internal directly, which is private.

Consider either:

• Adding a trusted_channel_features parameter to create_channel_to_trusted_peer_0reserve, or
• Adding a separate create_channel_to_trusted_peer that takes TrustedChannelFeatures directly (analogous to the accept side's API).